Configure additional routes for the subnets #128
Conversation
findmyname666
requested review from
tonynv,
andrew-glenn,
tlindsay42 and
a team
as code owners
| for az in local.azs : | ||
| [ | ||
| for route in var.subnets.public.routes : | ||
| merge(route, { "route_table_name" : az }) |
|
Overall this looks great to me. Question, what is the potential overlap / desired interaction if a user uses the predefined route configurations such as If they are mutually exclusive I might suggest writing a validation rule to prevent 😬 I'm aware that might be complicated but its worth considering Bravo btw. I had talked with the network SAs about this kind of feature for a while and knew it would be a bear to implement |
I was thinking about it, and in general, they are not mutually exclusive. However, there can be a race condition if, for example, the operator configures a custom route for |
Adding new optional attribute routes into variable subnets. This is useful in environments where core network, transit gateways, etc. are created out of operator control. Notes to the implementation: We are limited here by Terraform because for_each works only with set or map. As we have multiple subnets for a subnet / route table we cannot use it. For that reason the code creates list of maps where each map represents a route for the specific route table (attribute route_table_name). In such case we can use count to iterate over it. We are limited here by Terraform because for_each works only with a set or map. As we have multiple routes for a subnet / route table, we cannot use it. For that reason, the code creates a list of maps where each map represents a route for the specific route table (attribute route_table_name). In such case, we can use count to iterate over it.
findmyname666
force-pushed
the
subnets_routes
branch
from
f1ae03e to
a3818e8
Compare
|
Few ideas:
There is no formal method for depreciating vars afaik so best you can do is update the description and leave that for 3m then rip off the band-aid and release a new major version with a doc |
I think this would be "bad" from an user perspective in a case when NAT GW is created by this module, route
The same as above.
This is possible and shouldn't be difficult to implement. However the code would trigger the route recreation which can be unpleasant in the production environment. I see 1 more option. To add validation rule to ensure that there isn't route |
|
I guess i considered the validation rule to mean the inputs are mutually exclusive. so i think we agree on implementation strategies! :D nice validation too! LGTM |
…oute for 0.0.0.0/0 CIDR and connect_to_public_natgw set to true
findmyname666
force-pushed
the
subnets_routes
branch
from
aa33a01 to
50bc645
Compare
Adding new optional attribute
routesinto variablesubnets.This is useful in environments where core network, transit gateways, etc. are created out of operator control.
Notes to the implementation:
We are limited here by Terraform because
for_eachworks only with set or map. As we have multiple subnets for a subnet / route table we cannot use it. For that reason the code creates list of maps where each map represents a route for the specific route table (attributeroute_table_name). In such case we can usecountto iterate over it.We are limited here by Terraform because
for_eachworks only with a set or map. As we have multiple routes for a subnet / route table, we cannot use it. For that reason, the code creates a list of maps where each map represents a route for the specific route table (attributeroute_table_name). In such case, we can use count to iterate over it.